From: Jan Beulich Date: Fri, 9 Mar 2018 16:30:49 +0000 (+0100) Subject: cpufreq/ondemand: fix race while offlining CPU X-Git-Tag: archive/raspbian/4.11.1-1+rpi1~1^2~66^2~433 X-Git-Url: https://dgit.raspbian.org/%22http:/www.example.com/cgi/%22https:///%22http:/www.example.com/cgi/%22https:/?a=commitdiff_plain;h=185413355fe331cbc926d48568838227234c9a20;p=xen.git cpufreq/ondemand: fix race while offlining CPU Offlining a CPU involves stopping the cpufreq governor. The on-demand governor will kill the timer before letting generic code proceed, but since that generally isn't happening on the subject CPU, cpufreq_dbs_timer_resume() may run in parallel. If that managed to invoke the timer handler, that handler needs to run to completion before dbs_timer_exit() may safely exit. Make the "stoppable" field a tristate, changing it from +1 to -1 around the timer function invocation, and make dbs_timer_exit() wait for it to become non-negative (still writing zero if it's +1). Also adjust coding style in cpufreq_dbs_timer_resume(). Reported-by: Martin Cerveny Signed-off-by: Jan Beulich Tested-by: Martin Cerveny Reviewed-by: Wei Liu --- diff --git a/xen/drivers/cpufreq/cpufreq_ondemand.c b/xen/drivers/cpufreq/cpufreq_ondemand.c index fe6c63da8e..6b905d7cfc 100644 --- a/xen/drivers/cpufreq/cpufreq_ondemand.c +++ b/xen/drivers/cpufreq/cpufreq_ondemand.c @@ -204,7 +204,14 @@ static void dbs_timer_init(struct cpu_dbs_info_s *dbs_info) static void dbs_timer_exit(struct cpu_dbs_info_s *dbs_info) { dbs_info->enable = 0; - dbs_info->stoppable = 0; + + /* + * The timer function may be running (from cpufreq_dbs_timer_resume) - + * wait for it to complete. + */ + while ( cmpxchg(&dbs_info->stoppable, 1, 0) < 0 ) + cpu_relax(); + kill_timer(&per_cpu(dbs_timer, dbs_info->cpu)); } @@ -369,23 +376,22 @@ void cpufreq_dbs_timer_suspend(void) void cpufreq_dbs_timer_resume(void) { - int cpu; - struct timer* t; - s_time_t now; + unsigned int cpu = smp_processor_id(); + int8_t *stoppable = &per_cpu(cpu_dbs_info, cpu).stoppable; - cpu = smp_processor_id(); - - if ( per_cpu(cpu_dbs_info,cpu).stoppable ) + if ( *stoppable ) { - now = NOW(); - t = &per_cpu(dbs_timer, cpu); - if (t->expires <= now) + s_time_t now = NOW(); + struct timer *t = &per_cpu(dbs_timer, cpu); + + if ( t->expires <= now ) { + if ( !cmpxchg(stoppable, 1, -1) ) + return; t->function(t->data); + (void)cmpxchg(stoppable, -1, 1); } else - { - set_timer(t, align_timer(now , dbs_tuners_ins.sampling_rate)); - } + set_timer(t, align_timer(now, dbs_tuners_ins.sampling_rate)); } } diff --git a/xen/include/acpi/cpufreq/cpufreq.h b/xen/include/acpi/cpufreq/cpufreq.h index a5cd7d08a1..facbc14346 100644 --- a/xen/include/acpi/cpufreq/cpufreq.h +++ b/xen/include/acpi/cpufreq/cpufreq.h @@ -225,8 +225,8 @@ struct cpu_dbs_info_s { struct cpufreq_frequency_table *freq_table; int cpu; unsigned int enable:1; - unsigned int stoppable:1; unsigned int turbo_enabled:1; + int8_t stoppable; }; int cpufreq_governor_dbs(struct cpufreq_policy *policy, unsigned int event);